Conversation
|
|
||
| Artifacts represent infrastructure resources and connections in Massdriver. They can be provisioned by bundles or manually imported. | ||
|
|
||
| ## Commands |
There was a problem hiding this comment.
All these command blocks are redundant - Cobra already shows a list of available commands. Additionally many of them were out of date and missing new commands recently added.
| version | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
I just moved these in the file so that the sections were in alphabetical order.
There was a problem hiding this comment.
Pull request overview
This PR adds a new mass package reset command that allows users to reset a package status back to 'Initialized'. This is designed as an escape hatch for packages in unrecoverable states, such as being stuck in 'Pending' or unable to be decommissioned.
Key changes:
- Added new
ResetPackageAPI function that calls the resetPackage GraphQL mutation with hardcoded options (deleteDeployments: true, deleteState: false, deleteParams: false) - Implemented CLI command
mass package resetwith help documentation - Generated GraphQL bindings for the resetPackage mutation
Reviewed changes
Copilot reviewed 15 out of 23 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/api/package.go | Added ResetPackage function with hardcoded reset options |
| pkg/commands/pkg/reset.go | Added RunReset command handler that calls API reset |
| pkg/commands/pkg/reset_test.go | Added test for RunReset function |
| pkg/api/package_test.go | Added test for ResetPackage API function |
| cmd/package.go | Registered new pkgResetCmd with cobra CLI |
| pkg/api/zz_generated.go | Generated GraphQL types and functions for resetPackage mutation |
| pkg/api/genqlient.graphql | Added resetPackage GraphQL mutation definition |
| pkg/api/schema.graphql | Schema updates for integrations and other API changes |
| docs/helpdocs/package/reset.md | User documentation for reset command |
| docs/generated/mass_package_reset.md | Generated CLI documentation |
| docs/helpdocs/*.md | Removed command lists from documentation files |
| docs/generated/*.md | Updated generated documentation to reflect changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return gqlmock.MockMutationResponse("resetPackage", map[string]any{ | ||
| "id": "pkg-uuid1", | ||
| "slug": "ecomm-prod-cache", | ||
| "status": "ready", |
There was a problem hiding this comment.
The test expects the package status to be "ready", but according to the API documentation in pkg/api/zz_generated.go line 4711, the reset operation "will" set the package status to INITIALIZED. The test should expect "INITIALIZED" instead of "ready" to accurately reflect the behavior of the reset operation.
| if pkg.Slug != "ecomm-prod-cache" { | ||
| t.Errorf("got %v, wanted %v", pkg.Slug, "ecomm-prod-cache") | ||
| } | ||
| if pkg.Status != "ready" { |
There was a problem hiding this comment.
The test expects the package status to be "ready", but according to the API documentation, the resetPackage mutation sets the package status to INITIALIZED. The expected status should be "INITIALIZED" or use the PackageStatusInitialized constant instead of "ready".
| "result": map[string]any{ | ||
| "id": "pkg-uuid1", | ||
| "slug": "ecomm-prod-cache", | ||
| "status": "ready", |
There was a problem hiding this comment.
The test expects the package status to be "ready", but according to the API documentation, the resetPackage mutation sets the package status to INITIALIZED. The expected status should be "INITIALIZED" or use the PackageStatusInitialized constant instead of "ready".
| if pkg.Slug != "ecomm-prod-cache" { | ||
| t.Errorf("got %v, wanted %v", pkg.Slug, "ecomm-prod-cache") | ||
| } | ||
| if pkg.Status != "ready" { |
There was a problem hiding this comment.
The test expects the package status to be "ready", but according to the API documentation, the resetPackage mutation sets the package status to INITIALIZED. The expected status should be "INITIALIZED" or use the PackageStatusInitialized constant instead of "ready".
| deleteState := false | ||
| deleteParams := false | ||
| deleteDeployments := true |
There was a problem hiding this comment.
The ResetPackage function hardcodes deleteState, deleteParams, and deleteDeployments with no way for callers to customize these options. This limits flexibility for users who may want different reset behaviors. Consider either:
- Adding parameters to the function signature to allow callers to specify these options
- Adding a more flexible ResetPackageWithOptions function alongside this one
- Documenting why these specific defaults were chosen (particularly why deleteDeployments is true while the others are false)
| func runPkgReset(cmd *cobra.Command, args []string) error { | ||
| ctx := context.Background() | ||
|
|
||
| packageSlugOrID := args[0] | ||
|
|
||
| cmd.SilenceUsage = true | ||
|
|
||
| mdClient, mdClientErr := client.New() | ||
| if mdClientErr != nil { | ||
| return fmt.Errorf("error initializing massdriver client: %w", mdClientErr) | ||
| } | ||
|
|
||
| pkg, err := pkg.RunReset(ctx, mdClient, packageSlugOrID) | ||
| if err != nil { | ||
| return err | ||
| } | ||
|
|
||
| var name = lipgloss.NewStyle().SetString(pkg.Slug).Foreground(lipgloss.Color("#7D56F4")) | ||
| msg := fmt.Sprintf("✅ Package %s reset successfully", name) | ||
| fmt.Println(msg) | ||
|
|
||
| return nil | ||
| } |
There was a problem hiding this comment.
The package reset command performs a potentially destructive operation (deletes deployment history by default) without any confirmation prompt. Similar to the destroy command (lines 439-450), this command should prompt the user for confirmation before proceeding, unless a --force flag is provided. This is especially important since the operation deletes deployments and the API documentation warns this is "an escape hatch and can lose data".
pkg/commands/pkg/reset.go
Outdated
| "github.com/massdriver-cloud/massdriver-sdk-go/massdriver/client" | ||
| ) | ||
|
|
||
| // Resets a package with options to delete state, params, and deployments. |
There was a problem hiding this comment.
The comment states "Resets a package with options to delete state, params, and deployments" but the function doesn't actually provide any options - the values are hardcoded in the function body. The comment should be updated to accurately reflect that this function resets a package with deleteDeployments set to true and deleteState/deleteParams set to false, or the function should be enhanced to actually accept these options as parameters.
| // Resets a package with options to delete state, params, and deployments. | |
| // RunReset resets a package using the default reset behavior: | |
| // deleteDeployments=true, deleteState=false, deleteParams=false. |
No description provided.